Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(tls): detect modified CA and reissue certs #897

Merged
merged 2 commits into from
Jun 17, 2024

Conversation

ebaron
Copy link
Member

@ebaron ebaron commented Jun 15, 2024

Welcome to Cryostat! 👋

Before contributing, make sure you have:

  • Read the contributing guidelines
  • Linked a relevant issue which this PR resolves
  • Linked any other relevant issues, PR's, or documentation, if any
  • Resolved all conflicts, if any
  • Rebased your branch PR on top of the latest upstream main branch
  • Attached at least one of the following labels to the PR: [chore, ci, docs, feat, fix, test]
  • Signed all commits: git commit -S -m "YOUR_COMMIT_MESSAGE"

Fixes: #896

Description of the change:

  • Detects when updating our CA Issuer causes its secret name to change
  • If the secret has changed, delete any certificates owned by the CR (other than the CA), along with the secrets backing those certificates
  • The certificates and their secrets will be recreated by the existing reconciliation logic

Motivation for the change:

  • Fixes a bug where Cryostat was serving an old certificate after upgrading from 2.4 to 3.0. This could prevent users from accessing the Cryostat application.

How to manually test:

I tested on an OpenShift cluster, since the bug was very apparent when trying to access Cryostat using the route.

  1. Build and push an operator controller and bundle image for this PR. (I had to use Docker to make the bundle-upgrade work for some reason.)
    • Feel free to use mine: quay.io/ebaron/cryostat-operator-bundle:delete-cert-chain-01
  2. make deploy_bundle BUNDLE_IMG=quay.io/cryostat/cryostat-operator-bundle:2.4.0
  3. oc create -f config/samples/operator_v1beta1_cryostat.yaml
  4. Wait for Cryostat to be ready
  5. ./bin/operator-sdk run bundle-upgrade <bundle image for this PR>
  6. Wait for upgraded Cryostat to be ready
  7. curl -k -sSI https://cryostat-sample-cryostat-operator-system.apps.example.com
    HTTP/1.1 302 Found
    [...]
    

andrewazores
andrewazores previously approved these changes Jun 15, 2024
Copy link
Member

@tthvo tthvo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Cryostat is accessible now :D

@ebaron
Copy link
Member Author

ebaron commented Jun 17, 2024

/build_test

Copy link

/build_test completed successfully ✅.
View Actions Run.

Copy link
Contributor

@aali309 aali309 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@ebaron ebaron merged commit fd1becc into cryostatio:main Jun 17, 2024
7 checks passed
mergify bot pushed a commit that referenced this pull request Jun 17, 2024
* fix(tls): detect modified CA and reissue certs

* Ignore certificate not found when deleting

(cherry picked from commit fd1becc)

# Conflicts:
#	bundle/manifests/cryostat-operator.clusterserviceversion.yaml
ebaron added a commit that referenced this pull request Jun 17, 2024
* fix(tls): detect modified CA and reissue certs (#897)

* fix(tls): detect modified CA and reissue certs

* Ignore certificate not found when deleting

(cherry picked from commit fd1becc)

# Conflicts:
#	bundle/manifests/cryostat-operator.clusterserviceversion.yaml

* Fix conflicts

---------

Co-authored-by: Elliott Baron <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Bug] Cryostat certificate is signed by old CA after upgrade
4 participants